feat: add PostgreSQL EXCLUDE constraint parsing#2307
feat: add PostgreSQL EXCLUDE constraint parsing#2307fmguerreiro wants to merge 17 commits intoapache:mainfrom
Conversation
Per the Postgres `index_elem` grammar, an exclusion element may carry an operator class and ASC/DESC/NULLS FIRST|LAST qualifiers between the expression and the `WITH <operator>` tail. Add the two missing fields and route their display to the canonical position. Also simplify `ExclusionConstraint::span` by calling `Span::union_iter` directly and add `String` to the `no_std` import set so the crate continues to build with `--no-default-features`.
… ordering Three related fixes to the `EXCLUDE` table-constraint arm: - Guard the match on `PostgreSqlDialect | GenericDialect` so MySQL, SQLite, and others can continue to use `exclude` as a column name. Previously the arm fired on any dialect and hard-errored once the expected continuation was missing, instead of falling through to `parse_column_def`. - Extend `parse_exclusion_element` to parse the optional `opclass`, `ASC`/`DESC`, and `NULLS FIRST`/`LAST` qualifiers that precede `WITH <op>`, matching the PG `index_elem` grammar. - Add `parse_exclusion_operator` so the schema-qualified `OPERATOR(schema.op)` form is consumed as one unit. The previous single-token lookahead silently stopped at `OPERATOR` and left the parenthesised path to corrupt the surrounding parse.
Update the existing EXCLUDE tests to the current upstream APIs:
- `Statement::AlterTable` is a tuple variant wrapping `AlterTable`
- `AlterTableOperation::AddConstraint` is a struct variant with
`{ constraint, not_valid }`
- `Value::Number` takes `BigDecimal` under `--all-features`; use the
`number()` helper so the tests compile in CI's feature matrix
Expand coverage following upstream review:
- `NOT DEFERRABLE INITIALLY IMMEDIATE` complement to the existing
`DEFERRABLE INITIALLY DEFERRED` case
- Operator class: `col text_pattern_ops WITH =`
- Ordering qualifiers: `ASC NULLS LAST`, `DESC NULLS FIRST`
- Parenthesised function expression as element: `(lower(name))`
- Schema-qualified operator: `OPERATOR(pg_catalog.=)`
- Tighter error assertions on missing `WITH` and missing operator
- Negative test for non-PostgreSQL dialects (and smoke test that
`exclude` remains a legal column name in MySQL and SQLite)
Follow-up to the review feedback on the EXCLUDE constraint changes:
- Replace the hand-rolled `{ expr [opclass] [ASC|DESC] [NULLS ...] }`
lookahead inside `parse_exclusion_element` with a direct call to
`parse_order_by_expr_inner(true)` so the `index_elem` grammar lives
in a single place. `WITH FILL` is gated on a separate dialect
capability, so EXCLUDE (PG-only) cannot accidentally consume it.
- Add structural assertions to `parse_exclude_constraint_desc_nulls_first`
to mirror the ascending-order test instead of relying on the
round-trip alone.
- Assert that `exclude` survives as a column name in MySQL/SQLite by
checking the parsed AST rather than `is_ok()`.
- Tighten `exclude_empty_element_list_errors` and strengthen the
operator-class and function-expression tests with explicit `expr`
assertions for completeness.
- Document why `GenericDialect` is intentionally excluded from the
rejection sweep (it opts into PG-style EXCLUDE).
Eight pre-existing upstream lint violations in src/parser/mod.rs flagged by clippy::collapsible_match on the CI toolchain (rust 1.95.0). Each fix collapses an if block inside a match arm into a match guard. Locations fixed: - Line 512: Token::Word arm in parse_statements loop - Line 1309: Token::Word/SingleQuotedString arm in parse_wildcard_expr - Line 5035: Token::Word arm in parse_body_statements - Lines 8381/8398/8412/8426/8436: Hive row format delimiter arms
…' into feat/exclude-constraint-upstream
Replace the free-form `operator: String` on `ExclusionElement` with an `ExclusionOperator` enum distinguishing a single operator token from the Postgres schema-qualified `OPERATOR(schema.op)` form. Downstream visitors and rewriters can now pattern-match on the two cases instead of re-parsing a string. Factor the `OPERATOR(schema.op)` body out of the binary-operator path into a shared `parse_pg_operator_ident_parts` helper and reuse it from `parse_exclusion_operator` so the two call sites stay in lockstep. Add a `COLLATE` round-trip test for exclusion elements; `COLLATE` is consumed by the shared expression parser, so the new type exercises that flow end-to-end.
| /// `[ CONSTRAINT <name> ] EXCLUDE [ USING <index_method> ] ( <element> WITH <operator> [, ...] ) [ INCLUDE (<cols>) ] [ WHERE (<predicate>) ]` | ||
| /// | ||
| /// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE> | ||
| Exclusion(ExclusionConstraint), |
There was a problem hiding this comment.
| Exclusion(ExclusionConstraint), | |
| Exclude(ExcludeConstraint), |
does this make more sense if the constraint is called 'EXCLUDE' rather than 'EXCLUSION'?
| /// PostgreSQL `EXCLUDE` constraint. | ||
| /// | ||
| /// `[ CONSTRAINT <name> ] EXCLUDE [ USING <index_method> ] ( <element> WITH <operator> [, ...] ) [ INCLUDE (<cols>) ] [ WHERE (<predicate>) ]` | ||
| /// | ||
| /// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE> |
There was a problem hiding this comment.
| /// PostgreSQL `EXCLUDE` constraint. | |
| /// | |
| /// `[ CONSTRAINT <name> ] EXCLUDE [ USING <index_method> ] ( <element> WITH <operator> [, ...] ) [ INCLUDE (<cols>) ] [ WHERE (<predicate>) ]` | |
| /// | |
| /// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE> | |
| /// `EXCLUDE` constraint. | |
| /// | |
| /// `[ CONSTRAINT <name> ] EXCLUDE [ USING <index_method> ] ( <element> WITH <operator> [, ...] ) [ INCLUDE (<cols>) ] [ WHERE (<predicate>) ]` | |
| /// | |
| /// [PostgreSQL](https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE) |
| /// One element in an `EXCLUDE` constraint's element list. | ||
| /// | ||
| /// `{ column_name | ( expression ) } [ opclass ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] WITH <operator>` | ||
| /// | ||
| /// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE> |
There was a problem hiding this comment.
| /// One element in an `EXCLUDE` constraint's element list. | |
| /// | |
| /// `{ column_name | ( expression ) } [ opclass ] [ ASC | DESC ] [ NULLS { FIRST | LAST } ] WITH <operator>` | |
| /// | |
| /// See <https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE> | |
| /// One element in an `EXCLUDE` constraint's element list. | |
| /// | |
| /// [PostgreSQL](https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE) |
| #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] | ||
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] | ||
| pub enum ExclusionOperator { |
There was a problem hiding this comment.
| pub enum ExclusionOperator { | |
| pub enum ExcludeConstraintOperator { |
| } | ||
| } | ||
|
|
||
| /// The operator that follows `WITH` in an `EXCLUDE` element. |
There was a problem hiding this comment.
Can we add a link to the docs? Otherwise I think this struct wouldn't be very descriptive on its own
| OrderByExpr { | ||
| expr, | ||
| options: order, | ||
| .. | ||
| }, | ||
| operator_class, |
There was a problem hiding this comment.
this pattern looks a bit unusual, does it make more sense to instead call parse_order_by_expr_inner directly and store the IndexColumn instead of destructuring it in the ExclusionElement struct?
| /// | ||
| /// Accepts either a single operator token (e.g. `=`, `&&`, `<->`) or the | ||
| /// Postgres `OPERATOR(schema.op)` form for schema-qualified operators. | ||
| fn parse_exclusion_operator(&mut self) -> Result<ExclusionOperator, ParserError> { |
There was a problem hiding this comment.
| fn parse_exclusion_operator(&mut self) -> Result<ExclusionOperator, ParserError> { | |
| fn parse_exclude_constraint_operator(&mut self) -> Result<ExclusionOperator, ParserError> { |
|
|
||
| let operator_token = self.next_token(); | ||
| match &operator_token.token { | ||
| Token::EOF | Token::RParen | Token::Comma | Token::SemiColon => { |
There was a problem hiding this comment.
hmm could you explain this condition e.g. how does an optional RParen work?
|
|
||
| #[test] | ||
| fn roundtrip_exclude_constraint() { | ||
| let sql = "CREATE TABLE t (CONSTRAINT no_overlap EXCLUDE USING gist (room WITH =, during WITH &&) INCLUDE (id) WHERE (active = true))"; |
There was a problem hiding this comment.
a couple comments re the tests, for this scenario we already use verified_stmt in the other tests so that unless this is testing some specific syntax we can remove it (or rename it accordingly)?
Then can we merge the tests into the same function and just rely on verified statement for them (except maybe one simple scenario if we want to verify the ast)? that would keep the tests simpler
| { | ||
| let parser = TestedDialects::new(vec![dialect]); | ||
| assert!( | ||
| parser.parse_sql_statements(sql).is_err(), | ||
| "dialect unexpectedly accepted EXCLUDE: {sql}" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn exclude_as_column_name_parses_in_mysql_and_sqlite() { | ||
| // `exclude` must remain usable as an identifier where it is not a | ||
| // reserved keyword; PG reserves it as a constraint keyword. |
There was a problem hiding this comment.
for both tests, after introducing the dialect method I think we can move them to common.rs
Adds PostgreSQL
EXCLUDEtable constraint parsing.Reference: https://www.postgresql.org/docs/current/sql-createtable.html#SQL-CREATETABLE-EXCLUDE
Syntax
Operators may be a single token (
=,&&,<->) or the Postgresschema-qualified
OPERATOR(schema.op)form.Changes
src/ast/table_constraints.rs:ExclusionConstraint,ExclusionElement,ExclusionOperator(enum over token vsOPERATOR(…)form);TableConstraint::ExclusionwithDisplay,From,Spannedsrc/ast/mod.rs,src/ast/spans.rs: re-exports + spans armsrc/parser/mod.rs:EXCLUDEonPostgreSqlDialect | GenericDialectparse_exclusion_elementreusesparse_order_by_expr_inner(true)so the sharedindex_elemgrammar stays in one placeparse_pg_operator_ident_partsextracted from the binary-op path; reused byBinaryOperator::PGCustomBinaryOperatorandExclusionOperator::PgCustomtests/sqlparser_postgres.rs: 20 tests covering basic/multi-element, INCLUDE, WHERE, default index method, DEFERRABLE/NOT DEFERRABLE × INITIALLY DEFERRED/IMMEDIATE, opclass, ASC/DESC, NULLS FIRST/LAST, function-expression elements, COLLATE,OPERATOR(…)form, cross-dialect rejection (andexcluderemaining a legal column name in MySQL/SQLite), missing-WITH / missing-operator / empty-element-list errors